server/ai/provider/base.ts (1)

23-33: Redundant fallback chain for modelId.

Line 25 already ensures actualModelId falls back to resolvedModelId (which itself falls back to defaultFastModel), but line 33 adds another || defaultFastModel fallback. This is unreachable.

🧹 Simplify redundant fallback

-    const actualModelId = modelConfig?.actualName || resolvedModelId || defaultFastModel
+    const actualModelId = modelConfig?.actualName || resolvedModelId
     const requestedMaxTokens = params.max_new_tokens || 1024 * 8
     return {
       maxTokens:
         getEffectiveMaxOutputTokens(resolvedModelId, requestedMaxTokens) ??
         requestedMaxTokens,
       topP: params.top_p || 0.9,
       temperature: params.temperature || 0.6,
-      modelId: actualModelId || defaultFastModel,
+      modelId: actualModelId,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/ai/provider/base.ts` around lines 23 - 33, The returned modelId has an
unreachable fallback: `actualModelId` is already computed as
`modelConfig?.actualName || resolvedModelId || defaultFastModel`, so remove the
redundant `|| defaultFastModel` in the return object; update the return's
`modelId` to just use `actualModelId` (ensuring `resolvedModelId`,
`modelConfig`, `actualModelId`, and `defaultFastModel` remain unchanged).

server/tests/fetchModels.test.ts (1)

94-111: Consider adding test for missing model metadata entirely.

The test covers the case where the model exists but has null token limits. Consider adding a test for when getModelTokenLimits is called with a model ID that doesn't exist in the cache at all to verify the warning behavior.

🧪 Optional: Add test for completely missing model

test("warns and returns defaults for model not in cache", () => {
  __modelInfoInternals.setModelInfoCacheForTests([])

  const limits = getModelTokenLimits("nonexistent-model")
  expect(limits).toEqual({
    maxInputTokens: 128_000,
    maxOutputTokens: undefined,
  })
  // Optionally verify warning was logged if you have a way to capture logs
})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/tests/fetchModels.test.ts` around lines 94 - 111, Add a test that
verifies behavior when the model ID is not present in the cache: call
__modelInfoInternals.setModelInfoCacheForTests([]) then call getModelTokenLimits
with a nonexistent model ID and assert it returns the generic defaults
(maxInputTokens 128_000 and maxOutputTokens undefined); also optionally call
getEffectiveMaxOutputTokens with a sample requested output (e.g., 1500) to
assert it preserves the requested output and, if feasible, assert a warning was
emitted.

server/ai/fetchModels.ts (3)

456-468: Silent clamping of requestedMaxTokens without caller notification.

getEffectiveMaxOutputTokens silently clamps the requested value to maxOutputTokens using Math.min(). Callers passing values larger than the model's upstream maximum (e.g., 8192 when max is 4096) won't know their request was reduced.

Consider logging when clamping occurs or returning additional metadata.

📝 Optional: Add debug logging when clamping

 export const getEffectiveMaxOutputTokens = (
   modelId: Models | string | null | undefined,
   requestedMaxTokens?: number,
 ): number | undefined => {
   if (requestedMaxTokens === undefined) {
     return undefined
   }

   const { maxOutputTokens } = getModelTokenLimits(modelId)
-  return maxOutputTokens !== undefined
-    ? Math.min(requestedMaxTokens, maxOutputTokens)
-    : requestedMaxTokens
+  if (maxOutputTokens !== undefined && requestedMaxTokens > maxOutputTokens) {
+    Logger.debug(
+      { modelId, requestedMaxTokens, maxOutputTokens },
+      "[ModelInfo] Clamped requested max tokens to model limit."
+    )
+    return maxOutputTokens
+  }
+  return requestedMaxTokens
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/ai/fetchModels.ts` around lines 456 - 468, getEffectiveMaxOutputTokens
silently reduces requestedMaxTokens to the model's maxOutputTokens (from
getModelTokenLimits) which can surprise callers; update
getEffectiveMaxOutputTokens to notify when clamping occurs by either (a)
emitting a debug/process logger message that includes the original
requestedMaxTokens, the modelId, and the applied maxOutputTokens when Math.min
reduces the value, or (b) change the function signature to return metadata
(e.g., { effectiveMaxTokens, wasClamped, allowedMax }) so callers can react;
locate the logic in getEffectiveMaxOutputTokens and the getModelTokenLimits call
and implement one of these notification approaches consistently across usages.

---

72-74: Module-level mutable state without synchronization.

The modelInfoCache and warnedMissingTokenLimitKeys are module-level mutable variables. While JavaScript/TypeScript is single-threaded, concurrent async operations (e.g., multiple simultaneous fetchModelInfoFromAPI calls) could lead to race conditions where one call overwrites the cache while another is reading it.

Consider using a mutex pattern or ensuring only one fetch is in-flight at a time.

♻️ Proposed pattern to deduplicate in-flight requests

 let modelInfoCache: ModelInfoCache | null = null
+let inFlightFetch: Promise<RawModelInfoRecord[]> | null = null

 const warnedMissingTokenLimitKeys = new Set<string>()

Then in fetchModelInfoFromAPI:

 export async function fetchModelInfoFromAPI(
   forceRefresh = false,
 ): Promise<RawModelInfoRecord[]> {
   if (!forceRefresh && modelInfoCache) {
     const age = Date.now() - modelInfoCache.timestamp
     if (age < CACHE_TTL_MS) {
       return modelInfoCache.rawData
     }
   }

+  if (inFlightFetch) {
+    return inFlightFetch
+  }
+
+  inFlightFetch = (async () => {
+    try {
       // ... existing fetch logic ...
+    } finally {
+      inFlightFetch = null
+    }
+  })()
+
+  return inFlightFetch

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/ai/fetchModels.ts` around lines 72 - 74, modelInfoCache and
warnedMissingTokenLimitKeys are module-level mutable state that can race when
multiple fetchModelInfoFromAPI calls run concurrently; change the implementation
to deduplicate and synchronize in-flight fetches by storing an in-flight Promise
(e.g., inFlightModelInfoPromise) keyed by model id or a single mutex-like
Promise, so fetchModelInfoFromAPI checks for an existing in-flight Promise and
awaits it instead of starting a new network call, only updating modelInfoCache
and warnedMissingTokenLimitKeys once the Promise resolves; ensure any rejection
clears the in-flight entry so subsequent calls can retry and keep references to
the unique symbols modelInfoCache, warnedMissingTokenLimitKeys, and
fetchModelInfoFromAPI when locating the code to modify.

---

733-745: Test internals exported unconditionally.

The __modelInfoInternals object exposes internal functions and cache mutation helpers in production builds. While useful for testing, this could allow unintended cache manipulation.

🧪 Optional: Guard test exports with environment check

-export const __modelInfoInternals = {
+export const __modelInfoInternals = process.env.NODE_ENV === "test" ? {
   getCachedNormalizedModelMetadata,
   normalizeModelInfoRecords,
   resolveModelMetadata,
   resetModelInfoCacheForTests: () => {
     modelInfoCache = null
     warnedMissingTokenLimitKeys.clear()
   },
   setModelInfoCacheForTests: (records: RawModelInfoRecord[]) => {
     warnedMissingTokenLimitKeys.clear()
     updateModelInfoCache(records)
   },
-}
+} : {} as typeof __modelInfoInternals

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/ai/fetchModels.ts` around lines 733 - 745, The __modelInfoInternals
export currently exposes internal helpers (getCachedNormalizedModelMetadata,
normalizeModelInfoRecords, resolveModelMetadata) and test-only cache mutators
(resetModelInfoCacheForTests, setModelInfoCacheForTests) unconditionally;
restrict this by only assigning/exporting __modelInfoInternals when running in a
test environment (e.g., check process.env.NODE_ENV === 'test' or an isTestEnv
flag) so production builds do not expose modelInfoCache or
warnedMissingTokenLimitKeys mutation or the updateModelInfoCache helper; keep
the same object shape for tests but ensure the conditional guards the export
assignment.